Ensure tests do not leave signalhandlers #6667
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 10 files - 5 10 suites - 5 44m 50s ⏱️ - 5h 47m 46s For more details on these errors, see this check. Results for commit 9c37048. ± Comparison against base commit c992f80. ♻️ This comment has been updated with latest results. |
distributed/utils_test.py
Outdated
| _default_signals = None | ||
|
|
||
|
|
||
| def _set_default_signals(): |
There was a problem hiding this comment.
From this function name I assumed it was setting the process' signals with signal.signal, not collecting a copy of the expected signals
There was a problem hiding this comment.
does this work instead?
_default_signals = {sig: signal.getsignal(sig) for sig in signal.valid_signals()}
There was a problem hiding this comment.
>>> pprint.pp({sig: signal.getsignal(sig) for sig in signal.valid_signals()})
{<Signals.SIGHUP: 1>: <Handlers.SIG_DFL: 0>,
<Signals.SIGINT: 2>: <built-in function default_int_handler>,
<Signals.SIGQUIT: 3>: <Handlers.SIG_DFL: 0>,
<Signals.SIGILL: 4>: <Handlers.SIG_DFL: 0>,
<Signals.SIGTRAP: 5>: <Handlers.SIG_DFL: 0>,
<Signals.SIGABRT: 6>: <Handlers.SIG_DFL: 0>,
<Signals.SIGBUS: 7>: <Handlers.SIG_DFL: 0>,
<Signals.SIGFPE: 8>: <Handlers.SIG_DFL: 0>,
<Signals.SIGKILL: 9>: <Handlers.SIG_DFL: 0>,
<Signals.SIGUSR1: 10>: <Handlers.SIG_DFL: 0>,
<Signals.SIGSEGV: 11>: <Handlers.SIG_DFL: 0>,
<Signals.SIGUSR2: 12>: <Handlers.SIG_DFL: 0>,
<Signals.SIGPIPE: 13>: <Handlers.SIG_IGN: 1>,
<Signals.SIGALRM: 14>: <Handlers.SIG_DFL: 0>,
<Signals.SIGTERM: 15>: <Handlers.SIG_DFL: 0>,
16: <Handlers.SIG_DFL: 0>,
<Signals.SIGCHLD: 17>: <Handlers.SIG_DFL: 0>,
<Signals.SIGCONT: 18>: <Handlers.SIG_DFL: 0>,
<Signals.SIGSTOP: 19>: <Handlers.SIG_DFL: 0>,
<Signals.SIGTSTP: 20>: <Handlers.SIG_DFL: 0>,
<Signals.SIGTTIN: 21>: <Handlers.SIG_DFL: 0>,
<Signals.SIGTTOU: 22>: <Handlers.SIG_DFL: 0>,
<Signals.SIGURG: 23>: <Handlers.SIG_DFL: 0>,
<Signals.SIGXCPU: 24>: <Handlers.SIG_DFL: 0>,
<Signals.SIGXFSZ: 25>: <Handlers.SIG_IGN: 1>,
<Signals.SIGVTALRM: 26>: <Handlers.SIG_DFL: 0>,
<Signals.SIGPROF: 27>: <Handlers.SIG_DFL: 0>,
<Signals.SIGWINCH: 28>: <Handlers.SIG_DFL: 0>,
<Signals.SIGIO: 29>: <Handlers.SIG_DFL: 0>,
<Signals.SIGPWR: 30>: <Handlers.SIG_DFL: 0>,
<Signals.SIGSYS: 31>: <Handlers.SIG_DFL: 0>,
<Signals.SIGRTMIN: 34>: <Handlers.SIG_DFL: 0>,
35: <Handlers.SIG_DFL: 0>,
36: <Handlers.SIG_DFL: 0>,
37: <Handlers.SIG_DFL: 0>,
38: <Handlers.SIG_DFL: 0>,
39: <Handlers.SIG_DFL: 0>,
40: <Handlers.SIG_DFL: 0>,
41: <Handlers.SIG_DFL: 0>,
42: <Handlers.SIG_DFL: 0>,
43: <Handlers.SIG_DFL: 0>,
44: <Handlers.SIG_DFL: 0>,
45: <Handlers.SIG_DFL: 0>,
46: <Handlers.SIG_DFL: 0>,
47: <Handlers.SIG_DFL: 0>,
48: <Handlers.SIG_DFL: 0>,
49: <Handlers.SIG_DFL: 0>,
50: <Handlers.SIG_DFL: 0>,
51: <Handlers.SIG_DFL: 0>,
52: <Handlers.SIG_DFL: 0>,
53: <Handlers.SIG_DFL: 0>,
54: <Handlers.SIG_DFL: 0>,
55: <Handlers.SIG_DFL: 0>,
56: <Handlers.SIG_DFL: 0>,
57: <Handlers.SIG_DFL: 0>,
58: <Handlers.SIG_DFL: 0>,
59: <Handlers.SIG_DFL: 0>,
60: <Handlers.SIG_DFL: 0>,
61: <Handlers.SIG_DFL: 0>,
62: <Handlers.SIG_DFL: 0>,
63: <Handlers.SIG_DFL: 0>,
<Signals.SIGRTMAX: 64>: <Handlers.SIG_DFL: 0>}There was a problem hiding this comment.
pytest-timeout can use SIGALRM, the asyncio subprocess watcher MultiLoopChildWatcher and SafeChildWatcher can use SIGCHLD
There was a problem hiding this comment.
pytest-timeout can use SIGALRM, the asyncio subprocess watcher MultiLoopChildWatcher and SafeChildWatcher can use SIGCHLD
Question is when they'd install their handlers. I don't mind anybody using custom handlers but I want to make sure tests are not modifying this w/out cleanup (this is just paranoia), similar to our clean fixture
|
I'm inclined to not pick this up again. I was hoping for a quick win but it turns out this requires quite some tweaking to get running and I'm not convinced this is worth it |
Before #6205 CLI tests would install non-default signal handlers without removing them again by using the deprecated
install_signal_handlers.This was only possible for CLI tests that did not spawn a dedicated subprocess by using the Click
CLIRunner, e.g.test_idle_timeoutWith #6205 this was removed by introducing a task that is running
wait_for_signalsand is listening to signals that would restore the original signal handlers again.This autouse fixture ensure that we are guaranteed to not leave any dangling signal handlers and are using the default ones. If not, we'll see this as an error in the test suite instead of a spurious, unrelated test failure
cc @graingert @hendrikmakait @crusaderky